[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change #193501

alvarezmelissa87 · 2024-09-20T00:09:09Z

Summary

This PR

updates the LogRateAnalysisResultsTable to use EuiInMemoryTable to simplify sorting and pagination
adds sorting to Log rate change column
persists columns selected for viewing in the result view

Related meta issue: #187684

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

…lification and ease of sorting

x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_columns.tsx

elasticmachine · 2024-10-01T09:00:26Z

Pinging @elastic/ml-ui (:ml)

peteharverson

Tested latest changes and LGTM

walterra · 2024-10-01T15:52:35Z

The switch to in-memory-table causes a regression with syncing the table and histogram chart for hovering and pinning rows. On main the hovering/pinning acts on the items on display on the current table page only. This is no longer the case in this PR.

If nothing is hovered/pinned on the table, it should fall back to the first row of the current table page, highlight it and sync it with the histogram chart like this on main:

First page:

Second page:

In this PR it doesn't fall back to the first row:

On the second page of results, the fallback doesn't work again, because of all items passed on to the in-memory-table, there's still an item selected on the (invisible) first page and synced with the chart, so the histogram shown in the main chart doesn't match any item on the current page of the table:

… the contextual insight for observability

alvarezmelissa87 · 2024-10-01T19:19:14Z

@peteharverson, @walterra
In light of the regression that @walterra found (thank you!) I switched back to the basic table (as any optimizations we get with the in memory table would be negated by the need to manually keep track of pages of items for the hover/pinning behavior to be fixed).

In order to keep the way we sort consistent with the changes made for log rate analysis for the contextual insight for observability, this PR now adds the logRateChangeSort to the table items using the same logic.
When sorting by Log rate change the table will now be sorted by this field. The messaging will look the same.

@peteharverson - touched on this a bit with Walter but this logic is not in line with keeping the rate change higher than just docs up/down from 0. Keeping this as until further discussion on sorting so the sorting is consistent across the board.

peteharverson · 2024-10-02T08:47:58Z

.../aiops/public/components/log_rate_analysis_results_table/log_rate_analysis_results_table.tsx

@@ -123,6 +129,7 @@ export const LogRateAnalysisResultsTable: FC<LogRateAnalysisResultsTableProps> =
    const itemCount = significantItems?.length ?? 0;

    let items: SignificantItem[] = significantItems ?? [];
+    // console.log('--- SORT FIELD ---', sortField.props.children[0]);


Can this be removed?

Removed in 7f0c33b

peteharverson

Tested with latest changes and LGTM

walterra

To not rely on the REDUCTION_FACTOR, I'd like to suggest to change the sorting to use Number.POSITIVE_INFINITY instead of docCount when bgCount is zero. Then on top of that do a secondary sort on docCount. About the secondary sort I couldn't put an inline comment in the changed code because the place it needs to be done wasn't touched so far in this PR.

There is already a secondary sort in place for the p-value column, there we can add the other secondary sort. We need to do that for both ungrouped and grouped table here:

Regular table: https://github.com/elastic/kibana/blob/main/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/log_rate_analysis_results_table.tsx#L138
Grouped table: https://github.com/elastic/kibana/blob/main/x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/log_rate_analysis_results_table_groups.tsx#L281

walterra · 2024-10-02T12:53:08Z

x-pack/packages/ml/aiops_log_rate_analysis/get_log_rate_change.ts

@@ -9,6 +9,8 @@ import { i18n } from '@kbn/i18n';
 import { LOG_RATE_ANALYSIS_TYPE } from './log_rate_analysis_type';
 import type { LogRateAnalysisType } from './log_rate_analysis_type';

+const REDUCTION_FACTOR = 0.000001;


With my suggestion using Number.POSITIVE_INFINITY and a secondary sort on docCount I hope we can avoid relying on this sort of magic factor, can we remove it?

Updated in 7f0c33b and was able to remove that const.

walterra · 2024-10-02T12:53:27Z

.../aiops/public/components/log_rate_analysis_results_table/log_rate_analysis_results_table.tsx

+  const allSignificantItems = useMemo(() => {
+    return allItems.map((item) => ({
+      ...item,
+      logRateChangeSort: item.bg_count > 0 ? item.doc_count / item.bg_count : item.doc_count,


I'd like to suggest to use Number.POSITIVE_INFINITY instead of doc count when bg count is 0:

logRateChangeSort: item.bg_count > 0 ? item.doc_count / item.bg_count : Number.POSITIVE_INFINITY

Updated to use Number.POSITIVE_INFINITY in 7f0c33b

walterra · 2024-10-02T17:29:14Z

x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_columns.tsx

-          </>
-        ),
-        render: (_, { pValue }) => {
+        name: isGroupsTable ? GroupImpactColumnName : ImpactColumnName, // content={isGroupsTable ? groupImpactMessage : impactMessage}


Guess we can remove the comment?

walterra

Latest changes LGTM, also tested the sorting 👍

alvarezmelissa87 · 2024-10-02T22:27:09Z

@elasticmachine merge upstream

kibana-ci · 2024-10-03T00:13:18Z

💚 Build Succeeded

Buildkite Build
Commit: f7112fa

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`aiops`	551.9KB	552.7KB	+845.0B

Unknown metric groups

API count

id	before	after	diff
`@kbn/ml-agg-utils`	94	95	+1

History

💛 Build #238901 was flaky 7f0c33b
💔 Build #238541 failed 559451f
💔 Build #238214 failed c034dc5

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @alvarezmelissa87

kibanamachine · 2024-10-03T16:40:27Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11166038208

…nge (elastic#193501) ## Summary This PR - updates the `LogRateAnalysisResultsTable` to use `EuiInMemoryTable` to simplify sorting and pagination - adds sorting to `Log rate change` column - persists columns selected for viewing in the result view Related meta issue: elastic#187684 ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit c18184a)

kibanamachine · 2024-10-03T16:44:52Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…te change (#193501) (#194862) # Backport This will backport the following commits from `main` to `8.x`: - [[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change (#193501)](#193501)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Melissa Alvarez <[email protected]>

…nge (elastic#193501) ## Summary This PR - updates the `LogRateAnalysisResultsTable` to use `EuiInMemoryTable` to simplify sorting and pagination - adds sorting to `Log rate change` column - persists columns selected for viewing in the result view Related meta issue: elastic#187684 ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Elastic Machine <[email protected]>

wip: convert logRateAnalysis result table to in memory table for simp…

b701e27

…lification and ease of sorting

alvarezmelissa87 self-assigned this Sep 20, 2024

alvarezmelissa87 added 2 commits September 20, 2024 16:28

ensure log rate change and impact can be sorted correctly

2866b78

persist columns selected for display

25e6ba7

alvarezmelissa87 mentioned this pull request Sep 20, 2024

[ML] AIOps: 8.16 Log Rate Analysis improvements. #187684

Closed

alvarezmelissa87 requested review from walterra and peteharverson September 20, 2024 22:58

peteharverson reviewed Sep 23, 2024

View reviewed changes

x-pack/plugins/aiops/public/components/log_rate_analysis_results_table/use_columns.tsx Outdated Show resolved Hide resolved

alvarezmelissa87 added 2 commits September 30, 2024 11:13

Merge branch 'main' into ml-log-rate-change-column

3a58227

ensure sorting works correctly

c034dc5

alvarezmelissa87 marked this pull request as ready for review September 30, 2024 22:19

alvarezmelissa87 requested a review from a team as a code owner September 30, 2024 22:19

peteharverson added the :ml label Oct 1, 2024

peteharverson added Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v9.0.0 v8.16.0 backport:version Backport to applied version labels labels Oct 1, 2024

peteharverson approved these changes Oct 1, 2024

View reviewed changes

fix regression and keep sorting consistent with log rate analysis for…

559451f

… the contextual insight for observability

alvarezmelissa87 added release_note:enhancement backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 1, 2024

peteharverson reviewed Oct 2, 2024

View reviewed changes

peteharverson approved these changes Oct 2, 2024

View reviewed changes

walterra reviewed Oct 2, 2024

View reviewed changes

update sorting logic

7f0c33b

walterra reviewed Oct 2, 2024

View reviewed changes

walterra approved these changes Oct 2, 2024

View reviewed changes

Merge branch 'main' into ml-log-rate-change-column

f7112fa

alvarezmelissa87 merged commit c18184a into elastic:main Oct 3, 2024
20 checks passed

alvarezmelissa87 deleted the ml-log-rate-change-column branch October 3, 2024 16:40

kibanamachine mentioned this pull request Oct 3, 2024

[8.x] [ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change (#193501) #194862

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change #193501

[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change #193501

alvarezmelissa87 commented Sep 20, 2024 •

edited by kibanamachine

Loading

elasticmachine commented Oct 1, 2024

peteharverson left a comment

walterra commented Oct 1, 2024

alvarezmelissa87 commented Oct 1, 2024 •

edited

Loading

peteharverson Oct 2, 2024

alvarezmelissa87 Oct 2, 2024

peteharverson left a comment

walterra left a comment

walterra Oct 2, 2024

alvarezmelissa87 Oct 2, 2024

walterra Oct 2, 2024

alvarezmelissa87 Oct 2, 2024

walterra Oct 2, 2024

walterra left a comment

alvarezmelissa87 commented Oct 2, 2024

kibana-ci commented Oct 3, 2024

API count

kibanamachine commented Oct 3, 2024

kibanamachine commented Oct 3, 2024

[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change #193501

[ML][AIOps] Log rate analysis: ensure ability to sort on Log rate change #193501

Conversation

alvarezmelissa87 commented Sep 20, 2024 • edited by kibanamachine Loading

Summary

Checklist

elasticmachine commented Oct 1, 2024

peteharverson left a comment

Choose a reason for hiding this comment

walterra commented Oct 1, 2024

alvarezmelissa87 commented Oct 1, 2024 • edited Loading

peteharverson Oct 2, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Oct 2, 2024

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

walterra Oct 2, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Oct 2, 2024

Choose a reason for hiding this comment

walterra Oct 2, 2024

Choose a reason for hiding this comment

alvarezmelissa87 Oct 2, 2024

Choose a reason for hiding this comment

walterra Oct 2, 2024

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

alvarezmelissa87 commented Oct 2, 2024

kibana-ci commented Oct 3, 2024

💚 Build Succeeded

Metrics [docs]

Async chunks

API count

History

kibanamachine commented Oct 3, 2024

kibanamachine commented Oct 3, 2024

💚 All backports created successfully

Questions ?

alvarezmelissa87 commented Sep 20, 2024 •

edited by kibanamachine

Loading

alvarezmelissa87 commented Oct 1, 2024 •

edited

Loading